-
Notifications
You must be signed in to change notification settings - Fork 36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add global Celeritas input definition #1562
Conversation
Test summary 4 478 files 6 904 suites 15m 23s ⏱️ Results for commit e5c891f. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a little confused about how this input fits into building celeritas. It seems like there's quite a bit of logic between the inp
structs and what would get passed directly to Params
.
My personal imagining is that there's a layer of purely input / user configuration that gets canonicalized into a single format that is used to initialize celeritas. Inputs from prebuilt binaries like celer-sim
and celer-g4
would have default and easy ways to build the canonical input. If users link celeritas as a library and want to hook in to alter the defaults and force their desired configurations, then they can do so in between the "build input" level and the "initialize celeritas" level. This could very much be me misunderstanding the purpose / existing state of the initialization as well.
I've been thinking about this a bit more too. Let's say the goal is to have all the input data, aside from the in-memory geometry definition (which we do have an issue to create a standalone 'model' from). We have I think three kinds of inputs:
I'll be thinking more about this before our meeting this afternoon... |
@hhollenb Based on our discussions: thoughts? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good so far - just some small comments!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @sethrj! I think there still might be one or two tiny comments that slipped through the cracks? But looks great on my end!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great @sethrj . Thanks for all the reorganization work.
UPDATE: this is ready to go. For a preview of the documentation, see https://sethrj.github.io/celeritas/user/usage/input.html and https://sethrj.github.io/celeritas/user/implementation/core-physics/problem-def.html#setting-up-problems . For an example of how the input will be used, see 769fb8b .
To cleanly reuse the same input across all front ends, I've refactored the input into:
There's a lot of code changes, but most of it is moving stuff around and adding "adapter" code. There is also a lot of documentation.
Currently the code is "orphaned" i.e. it doesn't do anything useful; the next stage will be to merge the shared params setup code from the three front ends into a single place in
celeritas/setup/
.Related to #1556 , I'd like to specify a unified mechanism for creating core params, states, etc. before continuing work on that front. Simultaneously (in light of the Optical work) we've been discussing a "refactored" IO where we specify Celeritas inputs to classes separately from the classes themselves. This ties in with #1204 and #1263 and the unification of
RunInput
/RunnerInput
/SetupOptions
.As a first step, I've defined a new directory and namespace
inp
where we can move all the "input"/"option" classes into. For now we can hand-roll these in C++ (and adapt them from existing input classes), but I'd like to keep working on celeritas-project/celerpy#4 . We should get some working C++ structs before writing the python code and adapters, but ChatGPT seems able to help here, since it was able to generate this from the newinp::Tuning
C++ class:I used ChatGPT to help with the initial translation of SetupOptions, RunInput, and RunnerInput to the new unified
inp::Input
class.Note that for now we will need to restrict the along-step field choices to our built-in ones. I'd like to extend so that we can use more type-safe attributes (e.g.
ParticleId
) along @hhollenb 's line of thinking.If this sounds good, I'll add documentation to the manual about how I expect this will be used. Follow-on PRs will:
inp
dataFooParams::Input
,FooParams::Option
,FooOptions
, etc. intoinp
structuresTalking this through with @hhollenb I think we'll want to separate "standalone" driver functionality (primary generator definition) from the rest of it.
Goals
There are three categories of input types:
Reduce duplication of CoreParams construction and front end input
Currently celer-sim, celer-g4, and direct framework coupling all have slightly different interfaces. They also all independently construct CoreParams. I want to be able to use all the built-in functionality of Celeritas from any front end.
Reduce duplication of input parameters
Currently for celer-g4, we have (using
linear_loss_limit
as an example):This is a crazy amount of duplication and hard to keep track of where parameters come from and go. The duplication makes it harder to extend: to add support for region/particle-dependent limits we'd have to change a lot of code.
Unify and refactor input to classes
A data-oriented approach will let us restructure e.g. Sim/TrackInit params, as well as Physics, since the input can be pulled from multiple input structs. It also provides a single location where input structs are defined, rather than scattered throughout the codebase. That will ease the transition to a JSON front end and remove a lot of the assignment of
foo.x = bar.x
between input classes.Enable well-defined extension points
The current generic
SetupOptions
only gives one way to extend the actions:make_along_step
. Instead we want to be able to extend actions, along-step, physics processes, etc.